-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: reconcile policy webhook configurations #576
feat: reconcile policy webhook configurations #576
Conversation
d8925f6
to
6c6021b
Compare
0c4cdbc
to
4041d1a
Compare
64eefa4
to
0909b3f
Compare
0118d13
to
84be709
Compare
6bc7be6
to
7381c5d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Good way of ensuring the webhooks stay as they should, plus the tests against a real cluster open a world of possibilities ❤️.
…oncile if changed Signed-off-by: Fabrizio Sestito <fabrizio.sestito@suse.com>
…dmissionPolicy and ClusterAdmissionPolicy controllers Signed-off-by: Fabrizio Sestito <fabrizio.sestito@suse.com>
…luster using a k3s testcontainer Signed-off-by: Fabrizio Sestito <fabrizio.sestito@suse.com>
tests Signed-off-by: Fabrizio Sestito <fabrizio.sestito@suse.com>
Signed-off-by: Fabrizio Sestito <fabrizio.sestito@suse.com>
Signed-off-by: Fabrizio Sestito <fabrizio.sestito@suse.com>
Signed-off-by: Fabrizio Sestito <fabrizio.sestito@suse.com>
4efdcc9
to
12c7b72
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, this is some great work 👏
Description
This PR fixes: #224, adding reconciliation of ValidatingWebhookConfiguration and MutatingWebhookConfiguration if those are changed (by the user).
Test
This PR introduces a k3s
testcontainer
to test the controllers against a real cluster, by usingenvtest
.The
envtest
setup was already there, but not using a real cluster we weren't able to test Kubernetes builtin resources "side-effects", e.g. creating a deployment was not causing a ReplicaSet and Pods to be created. This is because by defaultenvtest
spins up a fake control plane with no Kubernetes built-in controllers.Also, we were lacking tests for the admission policies controllers' happy paths. This PR adds them.
NOTE: Webhooks integration tests are still not running against a real cluster, but using
envtest
in this case might be overkill.Consider refactoring these tests to unit tests (see: #579)